-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cpu/sam0_common: implement QSPI peripheral, add support to mtd_spi_nor #15300
base: master
Are you sure you want to change the base?
Conversation
03f72f4
to
95d1fd4
Compare
Does it make sense to split 2a3a56d to a separate PR so that we have somewhere to discuss the QSPI api? |
Just the API? |
It might make sense to also split out the SPI NOR changes at some point so that we can write multiple implementations for QSPI in parallel. |
* driver has a maximal transfer size. | ||
* negative error | ||
*/ | ||
ssize_t qspi_read(qspi_t bus, uint32_t addr, void *data, size_t len); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit puzzled by this interface. I think it is mixing the flash concepts (board or driver defined) with the qspi concepts (cpu defined). I don't think the cpu's qspi module should have knowledge about 0x6B being a read command. This depends on the mode you have configured on the flash and varies between flash chips.
QSPI read operation means that all four IO0~3 are controller-driven for a certain number of clocks (opcode + 0 to 4 address bytes) and then they are peripheral-driven for the remaining number or clocks. This support is what I have seen in various cpus.
Depending on the flash chip, flash chip mode or the opcode the address would be also in quad mode, but I don't think that's something this interface should deal with. The flags
in configure function would probably need to have the address-length, quad vs dual mode, and the number of dummy bytes, which also changes at runtime because of the different modes/commands you can send to a flash (not all commands are just fast_reads or fast_writes).
I'd propose for QSPI operation to have only these two instead of the 4 functions here:
ssize_t qspi_read(qspi_t bus, uint8_t cmd, uint32_t addr, void *data, size_t len);
ssize_t qspi_write(qspi_t bus, uint8_t cmd, uint32_t addr, const void *data, size_t len);
These functions could also serve as the single-mode operation using the 1bit mode in the flags. I didn't see full-duplex like in regular SPI blocks (read from IO1 while write to IO0) supported in the QN9080 or nRF52840. Howver, at least a function that lets you do send N bytes in single mode and then receive M bytes would be very useful, again this could be just qspi_read with the proper flags. In QN9080 I think N<=5 and in NRF52840 N<=8 is possible, and it is needed to send other commands.
#endif | ||
|
||
/** | ||
* @brief Configuration Options for QSPI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add here number of dummy cycles after the address, more address length options (1-4 bytes) and whether the address is also transmitted in quad mode.
#define QSPI_FLAG_1BIT (0x0) /**< single data line */ | ||
#define QSPI_FLAG_2BIT (0x1) /**< two parallel data lines */ | ||
#define QSPI_FLAG_4BIT (0x2) /**< four parallel data lines */ | ||
#define QSPI_FLAG_8BIT (0x3) /**< eight parallel data lines */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means IO0 to IO7 pins? Do you have an example CPU that supports this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
STM32H7 supports Octo-SPI 🐙
/** | ||
* @brief Serial Flash JEDEC commands | ||
*/ | ||
typedef enum { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this belongs here. In the NOR case this is not in the cpu's spi case, and we could use this module with chips that don't necessarily conform to these registers.
* @param[in] flags QSPI Configuration Options | ||
* @param[in] clk_hz QSPI frequency in Hz | ||
*/ | ||
void qspi_configure(qspi_t bus, qspi_mode_t mode, uint32_t flags, uint32_t clk_hz); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
qn9080 and nrf52840 both support memory mapping a flash to a region. To do that you need to program a command (like the fast_read command) and an address format, so then you can read from MCU memory directly which will generate a QSPI transaction automatically, even with some caching, allowing very fast reads without any call overhead. This is often incompatible with running a command at the same time, so something like qspi_configure but for configuring memory reads that returns the pointer/size would be good to add here. It is probably an addition to this module, meaning that you would need to have MODULE_QSPI_MAPPING defined, but it is worth considering the idea in this design so we don't have to change it later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SAM D5x/E5x has that too, but the memory region where this is mapped to is fixed.
I have a branch where I implemented qspi_xip_mount()
- haven't pushed that yet because I couldn't get the flash chip out of XIP mode without doing a power reset.
I assumed that in this mode you can only read, not write to the flash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading the datasheet it seems that you can only read, but it is not very clear. The nRF52840 has registers for configuring the READ, WRITE and ERASE opcodes, but I can't quite tell if the chip handles writes or not. QN9080 says it only handles reads and actually XIP is not recommended because it is too slow.
I think this is another example of code that shouldn't go into the qspi cpu driver: sending commands to enabled/disable the xip mode in the flash chip (looking at your code I understand you mean no-opcode mode). Some chips use 0xa5 for entering this mode and 0xff to exit, but this depends on the flash chip, not the cpu, and it varies a bit across manufacturers. For this what's needed is a way to set the dummy bytes to send (probably in the flags, or similar).
The XIP mount memory region is normally a window of certain fixed size and address; but you could have a different base flash address if the flash is larger than the window. I don't see this feature in the samd5x, but nrf52840 has the XIPOFFSET register for this purpose. I think that when configuring the memory mapping (xip_mount
in your branch) the caller should pass the configuration: command (if any), whether an command is needed, the flash base address; and the driver would return the memory address corresponding to the requested base address. If windowing is supported you would set that in the xipoffset register for example, but if there are alignment requirements for this offset or having an offset is not supported the driver could return an address into the fixed region (like AHB_XIP+base_address
or AHB_XIP + base_address % min_alignment
while setting the offset to another register). The caller in this example would be a qspi_nor driver, which is different from a qspi_flash driver for example but the same across CPUs. I'd imagine that the qspi_nor driver can send commands to enable the XIP mode in the flash (depending on the flash) and then tell the qspi.h interface to do xip_mount with no opcode; or don't set the XIP mode in the flash if not supported and then tell the qspi.h to do xip_mount using certain opcode for reads.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions. |
This still seems to be useful and would be a nice addition if merged. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions. |
Contribution description
This implements the QSPI peripheral found on the samd5x family and extends the
mtd_spi_nor
driver to make use of it.The QSPI peripheral can only be used on a fixed set of pins, but it doesn't use any SERCOM.
same54-xpro
comes with a 256 MBit flash that is configured as a MTD device with this PR.Right now this uses the QSPI in memory mapped mode. I just copied that part from the Adafruit_SPIFlash library.
The memory is mapped from
0x04000000
up to0x05000000
.Unfortunately it seems like the flash start address of that mapping can not be changed, so we are limited to 16 MiB.
Alternatively we can operate the device in SPI mode and read / write data to the TXDATA/RXDATA registers using DMA.
That way we should be able to address the whole memory.
TOOD
same54-xpro
can be used. It should be possible to use non-memory mapped mode and DMA to work around this.Testing procedure
tests/periph_qspi
provides a low-level test which can be used to read / write / erase the QSPI flash directlyexamples/filesystem
or any of the file system tests that use the MTD subsystem should workIssues/PRs references